Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation and delta-ui update #8

Merged
merged 8 commits into from
Apr 13, 2022
Merged

Documentation and delta-ui update #8

merged 8 commits into from
Apr 13, 2022

Conversation

danielfdsilva
Copy link
Collaborator

  • Restructure documentation splitting it into several files to separate concerns
  • Update the delta-ui to the latest main
  • Update the deploy url to https://visex.surge.sh/

@hanbyul-here @ricardoduplos Please check that the documentation makes sense and there are no mistakes. Feel free to reword section if they are not clear.

Once this is merged to deploy, the app will be automatically deployed to the surge url.

Copy link
Collaborator

@hanbyul-here hanbyul-here left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some small edits, but I don't seem to have write access :[ Those changes I made are mainly just small clarifications, so I will hold them for now.

  • Not a blocker but some things that I'd like to think further
  • some frontmatter documentations are in docs/CONTENT.md, others are in docs/frontmatter. It is well inked, not a problem at all reading the documentation through, but I wonder it won't be convenient to look up later.

@@ -13,6 +13,9 @@ API_RASTER_ENDPOINT='https://staging-raster.delta-backend.xyz'
# Endpoint for the STAC server. No trailing slash.
API_STAC_ENDPOINT='https://staging-stac.delta-backend.xyz'

# The mapbox public token obtained from your account
MAPBOX_TOKEN='pk.eyJ1IjoiY292aWQtbmFzYSIsImEiOiJja2F6eHBobTUwMzVzMzFueGJuczF6ZzdhIn0.8va1fkyaWgM57_gZ2rBMMg'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe to commit to the code base? (The repo might be private, but the build will be publicly accessible, right?)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the token is going to be exposed inevitably (user can see it through network request) but Mapbox token can be protected through url restriction (which can be set up through their dashboards.) I can't confirm how this key is set up now, but this is the same one in the ui repo https:/NASA-IMPACT/delta-ui/blob/main/.env#L13 so I don't think it necessarily decreases the safety, but we probably can find a better place for these keys like somewhere in ci pipeline.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. As long as it's not a security issue I'll merge this and then we can address it later

@leothomas leothomas merged commit c47460e into develop Apr 13, 2022
@leothomas leothomas deleted the feature/docs branch April 13, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants